Feature: add proxmox_node_network and proxmox_node_network_info modules #153
Conversation
aleskxyz
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #153 +/- ##
==========================================
+ Coverage 64.42% 68.19% +3.76%
==========================================
Files 64 68 +4
Lines 6530 7870 +1340
Branches 1257 1639 +382
==========================================
+ Hits 4207 5367 +1160
- Misses 2147 2271 +124
- Partials 176 232 +56 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
aleskxyz
force-pushed
the
add-network-module
branch
from
e566edd to
5260015
Compare
|
@gyptazy Would you have time to review this? I'm not well-versed enough in Python to review this :-( |
Will do but can't promise to do it this weekend. Probably on Monday. |
| """Validate IPv4 CIDR format.""" | ||
| if not cidr: | ||
| return False | ||
| pattern = r"^(\d{1,3}\.){3}\d{1,3}/\d{1,2}$" |
There was a problem hiding this comment.
Kudos for validating! But in that case we should probably to a more strict validating like:
_cidr_re = re.compile(
r'^(25[0-5]|2[0-4]\d|1\d{2}|[1-9]?\d)\.'
r'(25[0-5]|2[0-4]\d|1\d{2}|[1-9]?\d)\.'
r'(25[0-5]|2[0-4]\d|1\d{2}|[1-9]?\d)\.'
r'(25[0-5]|2[0-4]\d|1\d{2}|[1-9]?\d)'
r'/(3[0-2]|[12]?\d)$'
)
Maybe this is the point, where we should simply use libs like ipaddress instead of reinventing the wheel.
| """Validate IPv6 CIDR format.""" | ||
| if not cidr: | ||
| return False | ||
| pattern = r"^[0-9a-fA-F:]+/\d{1,3}$" |
There was a problem hiding this comment.
That might not be strict enough. Maybe this fits better:
r'^([0-9A-Fa-f:]+::?[0-9A-Fa-f:]*)/(12[0-8]|1[01][0-9]|[0-9]{1,2})$'
Also here, maybe better to use an external lib like ipaddress; even I'd like to avoid it.
There was a problem hiding this comment.
And since ipaddress is part of the standard library, using it would outsource complexity and not include major (installation) overhead. :)
Out of curiosity: Why would you like avoid including this library otherwise?
|
Hey @aleskxyz, that's well written and structured - appreciate your work! There're just some smaller things that're not yet important (but might need some changes in future releases) but it doesn't make sense or brings any huge benefits to adjust this right now. I didn't give OVS a try but apart from that it looks good to me. Just the regex might fail. I mean, it would be a user error but when we already have a validate function, we should do a proper validation (I added a quick idea, could probably also be improved). So far, well done! cc: @Thulium-Drake Cheers, |
aleskxyz
force-pushed
the
add-network-module
branch
from
e2613f7 to
5260015
Compare
- proxmox_node_network: Manage network interfaces (bridge, bond, eth, vlan, OVS) - proxmox_node_network_info: Retrieve network interface information - Support for staged configuration changes with apply/revert functionality - Full validation for interface-specific parameters - Comprehensive test coverage - Update runtime.yml to include new modules in proxmox action group - Support for parameter deletion
aleskxyz
force-pushed
the
add-network-module
branch
from
5260015 to
162921a
Compare
Hi @gyptazy |
|
@aleskxyz Thanks for your work! @gyptazy @IamLunchbox Are you happy with the results? :-) |
Also from me, thanks again for your work and sorry for the delay!
I’m currently abroad and will have a look at it asap. |
SUMMARY
Add comprehensive network interface management capabilities to the Proxmox collection with two new modules:
proxmox_node_networkfor managing network interfaces andproxmox_node_network_infofor retrieving network interface information. These modules support all major Proxmox network interface types including bridges, bonds, VLANs, and Open vSwitch interfaces with staged configuration management to prevent accidental network disruption.ISSUE TYPE
COMPONENT NAME
proxmox_node_network, proxmox_node_network_info
ADDITIONAL INFORMATION
New Modules Added:
proxmox_node_network: Manage network interfaces (bridge, bond, eth, vlan, OVS)proxmox_node_network_info: Retrieve network interface informationKey Features: